Skip to content

Comments

Feat/azure_ekm_proxy_api_version=0.1-preview#601

Open
HatemMn wants to merge 18 commits intodevelopfrom
feat/ekm_proxy_api
Open

Feat/azure_ekm_proxy_api_version=0.1-preview#601
HatemMn wants to merge 18 commits intodevelopfrom
feat/ekm_proxy_api

Conversation

@HatemMn
Copy link
Contributor

@HatemMn HatemMn commented Nov 18, 2025

Issue #653

Overview :

Basically an implementation of Azure EKM API as following in a loyal manner the specifications. This is version 0.1-preview, the code is flexible for future versions

What's done :

  • Added the necessary configs as well as the possibility to enable/disable azure EKM
  • Implemented the info / get metadata / Wrap / unwrap endpoints like specified
  • created all specified error responses, despite some of them being currently un-used
  • authentification ( mTLS )
  • Tests
  • About timeout : no more guards about it. The KMS does "his best"
  • Small refactoring with MS DKE

Otherwise :

  • the "1. Prerequisites" (read page 6) is not documented in the docs, users should refer to azure docs I guess

Closes #653

Appendix

Following are the docs diagrams
image

image

@HatemMn HatemMn self-assigned this Nov 18, 2025
@HatemMn HatemMn marked this pull request as ready for review November 22, 2025 12:43
@HatemMn HatemMn changed the base branch from develop to feat/support_RFC3394 January 9, 2026 17:03
@HatemMn HatemMn removed their assignment Jan 9, 2026
@HatemMn HatemMn added the Blocked Development can't advance until external conditions are met label Jan 9, 2026
@HatemMn
Copy link
Contributor Author

HatemMn commented Jan 9, 2026

Needs a rebase

Otherwise this will stop at this stage until an azure prod env is available to test

Base automatically changed from feat/support_RFC3394 to develop January 9, 2026 17:12
@Manuthor
Copy link
Contributor

Manuthor commented Jan 9, 2026

Needs a rebase

Otherwise this will stop at this stage until an azure prod env is available to test

I believe you can rebase from now since #658 has been merged to develop

@HatemMn HatemMn force-pushed the feat/ekm_proxy_api branch from 06af9e2 to 7cb2714 Compare January 12, 2026 07:45
@HatemMn
Copy link
Contributor Author

HatemMn commented Jan 12, 2026

Just rebased

@HatemMn HatemMn removed the Blocked Development can't advance until external conditions are met label Feb 3, 2026
@HatemMn HatemMn force-pushed the feat/ekm_proxy_api branch 2 times, most recently from 0d7c643 to 641d677 Compare February 9, 2026 16:14
# "target_cpu=native",
# ]

# can increase build time for system that support mold
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was added intentionally

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please remove this? If needed, add an internal README with specific build instructions

"non-fips"
]
],
"rust-analyzer.cargo.extraEnv": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was also added intentionally

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, remove it and add it to an internal README. IMHO, .vscode/settings.json should be removed completely.

feat: DONE  rfc5649
fix: fix problematic test
feat: finish up

fix: major bugé

feat: add a lot of things

fis: add back provider for ci tests

fix:clip

fix: a lot more stuff

fix: a lot more stuff2

fix: some fixes

fix: finish up
feat: reviews fixes + ui fixes + migrate fixes + a test

feat: missing file

fix: ui

fix: review fixes

fix: grammar fixes
feat: add azure ekm configs

feat: wip on errors

feat: finish start file

feat: big advance on metadata endpoint

feat: finish metadata but the code is ugly

refactor: HUGE refactoring of that huge nested code induced by the errors (I used handlers)

feat: more advance

feat: finish the api and fix compiler problems

feat: multiple improvements for endpoints

feat: more improvements
feat: auth OK

feat: auth seems ok ...?

feat: first refactor

fix: improve

fix: add missing files

fix: commit first files

fix: add the rest
fix: rfc algorithms

Revert "fix: rfc algorithms"

This reverts commit e5d9737.

fix: add rfc algos
feat: rfc3394 algo postfixes

feat: post review fixes and some new feats to AES habndlers

feat: final commits that got lost before finish

fix: test fixes
feat: finish docs and fix some more todos2
Comment on lines 38 to 41

# [target.x86_64-unknown-linux-gnu] # TODO: uncomment this after dev is finished
# linker = "clang"
# rustflags = ["-C", "link-arg=-fuse-ld=mold"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment to remember to revert this changes

"rust-analyzer.cargo.features": [
"non-fips"
]
],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert (and make it a user only configuration)

Ok(plaintext)
}

// // Encrypt block using AES with ECB mode i.e. raw AES as specified in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

# "target_cpu=native",
# ]

# can increase build time for system that support mold
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please remove this? If needed, add an internal README with specific build instructions

"non-fips"
]
],
"rust-analyzer.cargo.extraEnv": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, remove it and add it to an internal README. IMHO, .vscode/settings.json should be removed completely.

Comment on lines +24 to +32
/// WARNING: This bypasses mTLS authentication entirely. Only use for testing!
#[clap(
long,
env = "KMS_AZURE_EKM_DISABLE_CLIENT_AUTH",
default_value = "false"
)]
// serde does not support skipping booleans out of the box so a custom function is used
#[serde(skip_serializing_if = "is_false")]
pub azure_ekm_disable_client_auth: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this parameter really required?
If during testing, mTLS must be disabled, I would advise to disable this TLS configuration:

[tls]
# Your server certificate and private key (PKCS#12 format)
tls_p12_file = "/etc/cosmian/server-cert.p12"
tls_p12_password = "your-secure-password"

# The certificate downloaded in the previous section
# This validates the client certificate presented by Azure MHSM
clients_ca_cert_file = "/etc/cosmian/mhsm-root-ca.pem"

Isn't it enough?

Comment on lines +39 to +42
let status_code = e.status_code().as_u16();

// Mapping non-internal errors status numeric code to an error code string
let code = match status_code {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not convert to u16 and match directly on e.status_code()?

// Check algorithm and build response
match algorithm {
CryptographicAlgorithm::AES => {
if key_length == 256 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a const variable for this 256-value and mention on it that is a Azure ProxyEKM restriction.

)
})?;
let (modulus, public_exponent) =
get_rsa_key_metadata_from_public_key(kms, key_id, &user).await?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Watch out here.
Even your changes seem to be correct, there are currently no tests to validate if MS DKE encryption is broken or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support EKM Azure

2 participants